Claude finished @dbernstein's task in 8m 12s —— View job

PR Review: Keep one bad custom list from wedging the entries sweep (PP-4506)

• Read the diff (custom_lists.py, test_custom_lists.py)
• Trace where QueryParseException / RequestError are raised in the per-list path
• Verify the chord error-callback ("exactly one of the two runs") mechanism
• Check whether other list-specific exceptions (e.g. json.JSONDecodeError) can still wedge the sweep
• Review the new tests and the _wait_for_sweep_lock_release helper
• Post review

Summary

This is a solid, well-scoped fix and the tests are genuinely convincing. Both changes are correct: QueryParseException is raised by the JSONQuery parser inside wl.search (e.g. the published transform at search/query.py:791), so it belongs in the same list-specific "log-and-skip" bucket as an OpenSearch RequestError, and catching it restores the pre-pipeline behavior. The chord-body error-callback is the right tool for the lock leak — Celery invokes a chord body's link_error when a header task fails (via chord_error_from_stack), which is exactly the abort path that used to strand the sweep lock, and using .si correctly suppresses errback arg-injection so lock_value binds cleanly. finalize releasing via _sweep_lock(...).release() is value-scoped, so even in the unlikely event both the body and its errback fired, the second release is a harmless no-op rather than a correctness problem. I especially appreciated that test_failing_per_list_task_still_releases_sweep_lock exercises the real worker end-to-end with a genuine SQLAlchemyError and that you confirmed it isn't vacuous — that's the assertion that actually pins down the fix.

I looked specifically for a remaining hole where a bad row could still abort the sweep: populate_query_pages does its own json.loads(custom_list.auto_update_query) (core/query/customlist.py:131) in INIT/REPOPULATE mode, which raises a raw json.JSONDecodeError rather than QueryParseException. That turns out to be unreachable in practice — the admin controller only ever stores json.dumps(auto_update_query) (api/admin/controller/custom_lists.py:165), so the stored string is always valid JSON — so it's not worth widening the catch here. No blocking issues found; nothing to change.
· bugfix/custom-list-sweep-query-parse-error

Greptile Summary

This PR makes custom-list entry sweeps recover from bad per-list queries. The main changes are:

• Catch QueryParseException alongside OpenSearch RequestError.
• Register the sweep finalizer as the chord body's error callback.
• Add tests for malformed queries and lock release after failed per-list tasks.

Confidence Score: 4/5

The repopulate failure path needs a fix before merging.

• A malformed query during REPOPULATE can leave a custom list empty.
• The new lock-release behavior is covered by targeted tests.
• The parser-error tests do not cover the state that deletes entries before the parse failure.

src/palace/manager/celery/tasks/custom_lists.py

Important Files Changed

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Beat starts custom-list sweep] --> B{Acquire sweep lock?}
    B -- No --> C[Skip sweep]
    B -- Yes --> D[Run per-list update chord]
    D --> E{Per-list query malformed?}
    E -- Yes --> F[Log and skip that list]
    E -- Other task failure --> G[Chord error callback]
    E -- All tasks succeed --> H[Chord body]
    F --> H
    G --> I[Finalize releases sweep lock]
    H --> I

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Beat starts custom-list sweep] --> B{Acquire sweep lock?}
    B -- No --> C[Skip sweep]
    B -- Yes --> D[Run per-list update chord]
    D --> E{Per-list query malformed?}
    E -- Yes --> F[Log and skip that list]
    E -- Other task failure --> G[Chord error callback]
    E -- All tasks succeed --> H[Chord body]
    F --> H
    G --> I[Finalize releases sweep lock]
    H --> I

_{Reviews (1): Last reviewed commit: "Keep one bad custom list from wedging th..." | Re-trigger Greptile}

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.46%. Comparing base (7061612) to head (ad810db).
⚠️ Report is 12 commits behind head on main.

@@           Coverage Diff           @@
##             main    #3550   +/-   ##
=======================================
  Coverage   93.46%   93.46%           
=======================================
  Files         512      512           
  Lines       46614    46614           
  Branches     6352     6352           
=======================================
  Hits        43570    43570           
  Misses       1968     1968           
  Partials     1076     1076           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.